Replace @oslojs/webauthn + crypto sig verification with hand-rolled WebAuthn#1254
Replace @oslojs/webauthn + crypto sig verification with hand-rolled WebAuthn#1254ascorbic wants to merge 2 commits into
Conversation
…ebAuthn Hand-roll a strict, scoped WebAuthn parser and move ECDSA/RSA signature verification to WebCrypto, removing @oslojs/webauthn entirely and dropping @oslojs/crypto from the passkey path. - cbor.ts: strict CBOR decoder (definite-length maps/ints/byte strings only; rejects tags, floats, indefinite lengths, over-long inputs; depth-capped) - cose-key.ts / der.ts: COSE EC2/RSA -> stored SEC1/SPKI (unchanged formats) - authenticator-data.ts: authData/attestation/clientData parsing; validates extension data and asserts full-buffer consumption instead of ignoring it - verify.ts: crypto.subtle verification incl. ECDSA DER->raw conversion Stored key formats are unchanged so existing credentials keep working. Registration now records deviceType/backedUp from BE/BS flags, which the oslo parser did not expose. Tests use real attestation/assertion fixtures instead of mocking the parser. @oslojs/crypto stays for tokens.ts/oauth (sha256/hmac) -- out of scope.
- Fix extension-data regression: the scoped CBOR decoder now accepts the boolean/null simple values that real authenticator extensions emit (hmac-secret etc.), so ED-flagged assertions from hardware keys no longer fail login. Floats, tags, and indefinite-length items stay rejected. - CBOR: route the 8-byte integer low word through the bounds check so a truncated integer surfaces as CborError, not RangeError. - COSE: reject non-integer numbers as labels; bound RSA modulus/exponent size so a malicious 'none'-attestation authenticator can't register an oversized key that slows every later auth. - Add direct unit tests for cbor.ts and der.ts (malformed inputs, round-trips) and a regression test for boolean-extension assertions. Reviews found no auth-bypass/forgery/replay path; stored key formats verified byte-identical to the oslo output, so existing credentials keep working.
🦋 Changeset detectedLatest commit: ae166ed The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
PR template validation failedPlease fix the following issues by editing your PR description:
See CONTRIBUTING.md for the full contribution policy. |
Scope checkThis PR changes 1,299 lines across 16 files. Large PRs are harder to review and more likely to be closed without review. If this scope is intentional, no action needed. A maintainer will review it. If not, please consider splitting this into smaller PRs. See CONTRIBUTING.md for contribution guidelines. |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
docs | ae166ed | Jun 01 2026, 08:13 AM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-playground | ae166ed | Jun 01 2026, 08:13 AM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-demo-cache | ae166ed | Jun 01 2026, 08:14 AM |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/blocks
@emdash-cms/cloudflare
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
There was a problem hiding this comment.
Pull request overview
This PR removes the @oslojs/webauthn dependency and replaces the passkey (WebAuthn) parsing + signature verification path with a strict, hand-rolled WebAuthn/CBOR/COSE/DER implementation and crypto.subtle-based verification, while keeping stored credential key formats compatible (ES256 SEC1 uncompressed; RS256 SPKI DER).
Changes:
- Dropped
@oslojs/webauthnfrom the workspace and@emdash-cms/authpackage deps; adjusted lockfile and Vite prebundle includes accordingly. - Introduced strict CBOR decoding, authenticator data parsing (including extension consumption), COSE-key translation, minimal DER helpers, and WebCrypto verification primitives.
- Updated passkey registration/authentication flows and tests to exercise real attestation/assertion fixtures (including boolean-valued extensions).
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-workspace.yaml | Removes @oslojs/webauthn from the catalog. |
| pnpm-lock.yaml | Removes @oslojs/webauthn and its transitive entries from the lockfile. |
| packages/core/src/astro/integration/vite-config.ts | Drops prebundle includes for removed passkey dependencies. |
| packages/auth/src/passkey/verify.ts | Adds WebCrypto-based SHA-256 + assertion signature verification. |
| packages/auth/src/passkey/register.ts | Switches registration parsing/verification to new strict WebAuthn modules and WebCrypto RP ID hash check. |
| packages/auth/src/passkey/register.test.ts | Replaces mocked parsing with real CBOR-built attestation fixtures for ES256/RS256. |
| packages/auth/src/passkey/der.ts | Adds minimal DER helpers (RSA SPKI encoding, ECDSA DER→raw signature conversion). |
| packages/auth/src/passkey/der.test.ts | Adds unit tests for DER helpers and RSA SPKI round-tripping. |
| packages/auth/src/passkey/cose-key.ts | Adds COSE→stored-key conversion/validation for ES256 + RS256. |
| packages/auth/src/passkey/cbor.ts | Adds strict, scoped CBOR decoder used by WebAuthn parsing. |
| packages/auth/src/passkey/cbor.test.ts | Adds CBOR decoder unit tests (incl. boolean extension values). |
| packages/auth/src/passkey/authenticator-data.ts | Adds binary parsers for authenticatorData, attestation object, and clientDataJSON. |
| packages/auth/src/passkey/authenticate.ts | Switches authentication parsing/verification to new strict WebAuthn modules + WebCrypto verification. |
| packages/auth/src/passkey/authenticate.test.ts | Updates assertion signing/message building and adds a boolean-extension assertion test. |
| packages/auth/package.json | Removes @oslojs/webauthn dependency from @emdash-cms/auth. |
| .changeset/passkey-handrolled-webauthn.md | Documents the dependency removal and new strict parsing/verification behavior. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (typeof parsed !== "object" || parsed === null) { | ||
| throw new WebAuthnDataError("client data is not an object"); | ||
| } | ||
| const type = "type" in parsed ? parsed.type : undefined; | ||
| const challenge = "challenge" in parsed ? parsed.challenge : undefined; | ||
| const origin = "origin" in parsed ? parsed.origin : undefined; |
| * WebAuthn only ever hands us definite-length maps/arrays of integers, byte | ||
| * strings, and text strings (COSE keys, the attestation object, extension | ||
| * maps). This decoder supports exactly that and rejects everything else -- | ||
| * tags, floats, simple values, indefinite-length items, and anything that | ||
| * over-runs the buffer. A scoped parser is a smaller attack surface than a |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is a well-justified, narrowly-scoped replacement of @oslojs/webauthn and @oslojs/crypto signature verification in the passkey path with hand-rolled CBOR/WebAuthn parsers and WebCrypto primitives. The approach is sound: stored key formats are unchanged, the CBOR decoder is strictly limited to the types WebAuthn needs, and the WebCrypto verification path correctly handles the ECDSA DER→raw footgun. The adversarial-review notes in the description are reflected in the code (duplicate-key rejection, depth cap, RSA key-size bound, extension consumption).
I read the full diff, traced every changed hunk, and spot-checked call sites and sibling tests. The implementation is clean and follows AGENTS.md conventions (.js imports, type-only imports, changeset present).
One latent correctness issue in the DER encoder: encodeLength in der.ts uses JavaScript's signed right-shift (n >>= 8), which produces incorrect length bytes for values ≥ 2^31. This is unreachable with current RSA bounds (~1100 bytes max), but the utility should use Math.floor(n / 256) to remain correct.
Test coverage: authenticator-data.ts and cose-key.ts are exercised indirectly through register.test.ts and authenticate.test.ts, but direct unit tests for their edge cases (truncated auth data, malformed COSE keys, wrong key-type/algorithm combos) would strengthen the safety net for hand-rolled binary parsers. The existing 77 auth + 40 core passkey tests are a solid baseline.
Behavioral changes noted and accepted: malformed DER signatures now surface as invalid_signature instead of invalid_response (fail-closed, less information leakage); newly-registered passkeys correctly report deviceType/backedUp from authenticator flags instead of hardcoding singleDevice/false.
Verdict: comment — the implementation is correct and the approach is right, but fix the unreachable-yet-real DER length bug before landing.
| const total = parts.reduce((n, p) => n + p.length, 0); | ||
| const out = new Uint8Array(total); | ||
| let offset = 0; | ||
| for (const part of parts) { |
There was a problem hiding this comment.
[suggestion] n >>= 8 is a signed 32-bit right shift in JavaScript. For any length whose highest set bit is at position 31 or higher, the sign bit is preserved and the loop produces incorrect bytes (e.g. 0x80000000 shifts to 0xff800000, not 0x00800000).
While current callers never pass values that large (max RSA SPKI is ~1100 bytes), encodeLength is a general DER utility and the bug is subtle enough to bite if the bounds change or the function is reused elsewhere.
| for (const part of parts) { | |
| while (n > 0) { | |
| bytes.unshift(n & 0xff); | |
| n = Math.floor(n / 256); | |
| } |
github-actions
Bot
removed
the
review/needs-review
What
Replaces the
@oslojs/webauthndependency and@oslojs/crypto's ECDSA/RSA signature verification in the passkey path with a hand-rolled, strict WebAuthn parser plus WebCrypto-based verification.This is a response to a heads-up from the
@oslojsmaintainer (pilcrow), who flagged that he'd avoid these packages where possible:@oslojs/cryptofor signature verification (prefer an actively-maintained primitive — WebCrypto), and@oslojs/webauthnbecause its general-purpose CBOR decoder is broader than a passkey RP needs and it doesn't account for extension data in the authenticator data.Scope
Deliberately narrow — only the two things flagged:
@oslojs/webauthn→ removed entirely (it was only used in the passkey path). Catalog pin dropped.@oslojs/crypto→ removed from the passkey path, replaced withcrypto.subtle. It stays fortokens.ts/OAuth (sha256/hmac/constantTimeEqual), which are synchronous and not what was flagged — migrating those is a separate sync→async change.@oslojs/encoding(base64url) is kept — not flagged, trivial, not a security-parser concern.How it's built
cbor.tsauthenticator-data.tsder.tscose-key.tsverify.tscrypto.subtlesignature verification + SHA-256.Backward compatibility
Stored key formats are byte-for-byte unchanged (SEC1 uncompressed for ES256, SPKI DER for RS256) — verified against
@oslojs/crypto's actual output during review. Existing passkeys keep working; no migration.One behavioral improvement: newly-registered passkeys now record correct
deviceType/backedUpvalues from the authenticator's BE/BS backup flags, which the previous parser did not expose (it hardcodedsingleDevice/false).Verification
@emdash-cms/auth(real attestation/assertion fixtures — no parser mocking), incl. direct unit tests forcbor.ts/der.tsand a regression test for boolean-valued extensions (hmac-secret); 40 core passkey tests green.hmac-secrethardware keys, an 8-byte-intRangeErrorleak, and added an RSA key-size bound.Note for reviewers
The residual risk in hand-rolled WebAuthn is differential behavior vs real authenticators, not the security patterns (which are covered). Before this is battle-tested I'd want a corpus of real authenticator output (Chrome/Safari/a hardware key) exercised against the parser — the boolean-extension regression caught in review is the archetype of what unit tests with synthetic fixtures can miss.
Try this PR
Open a fresh playground →
A full working EmDash site, deployed from this branch. Each visit gets its own session-scoped sandbox: no login needed and no shared state. Try the admin, edit content, hit the public site.
Tracks
passkey-handroll-webauthn. Updated automatically when the playground redeploys.